fix(sandbox): remove DNS resolution from mechanistic mapper to prevent data exfiltration#1329
Conversation
…t data exfiltration The mechanistic mapper resolved denied hostnames via DNS to check whether they mapped to private IPs (for populating allowed_ips in proposals). This leaked the denied hostname through DNS queries even though the HTTP connection was blocked by policy — a data exfiltration vector. Remove resolve_allowed_ips_if_private() entirely. Proposals no longer include allowed_ips. If a user applies a proposed rule and the host resolves to a private IP, the proxy's SSRF defense denies the connection. That denial flows back through the aggregator, and the user can then explicitly configure allowed_ips in their policy. This two-step flow is also more explicit about private IP access, which is desirable given NVIDIA#1245. Closes NVIDIA#1169 Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
e7acd65 to
1de8e7a
Compare
johntmyers
left a comment
There was a problem hiding this comment.
Two small follow-ups from review:
-
In
crates/openshell-sandbox/src/mechanistic_mapper.rs, the SSRF security note still says approving the proposal “bypasses internal-IP safety checks.” Since proposals no longer includeallowed_ips, approving one does not bypass SSRF anymore. Consider changing this to something like: “This connection was blocked by SSRF protection. Private IP access requires an explicitallowed_ipspolicy entry.” -
architecture/security-policy.mdstill saysis_internal_ipis used by the mechanistic mapper to detect whenallowed_ipsshould be populated in proposals. That looks stale after this change. It should describeis_internal_ipas runtime proxy SSRF classification only, or otherwise note that mapper proposals intentionally do not populateallowed_ips.
Update the mechanistic mapper SSRF security note to reflect that proposals no longer populate allowed_ips — approving a proposal does not bypass SSRF protection. Clarify the two-step approval flow in architecture/security-policy.md. Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
|
Addressed both items from review: Item 1 — SSRF security note ( Item 2 — Architecture doc ( |
|
/ok to test 83eab3a |
Summary
The mechanistic mapper resolved denied hostnames via DNS to check whether they mapped to private IPs (for populating
allowed_ipsin proposals). This leaked the denied hostname through DNS queries even though the HTTP connection was blocked by policy — a data exfiltration vector.Related Issue
Closes #1169
Changes
resolve_allowed_ips_if_private()and all DNS resolution from the mechanistic mapperallowed_ips; private IP access requires explicit user configurationgenerate_proposals()from async to sync (no longer needs tokio for DNS)#[tokio::test] asyncto#[test]syncis_internal_iptests that belonged inopenshell-core, not the mapperIf a user applies a proposed rule and the host resolves to a private IP, the proxy's SSRF defense denies the connection. That denial flows back through the aggregator, and the user can then explicitly configure
allowed_ipsin their policy. This two-step flow is also more explicit about private IP access, which is desirable given #1245.Testing
mise run pre-commitpassesis_internal_iptests)Checklist